Fix invalid test.yml and broken fixture generation; add independent workflow lint gate - #93
Conversation
#92 left a duplicate `with:` key on the `react-lint` checkout step, which makes test.yml unparseable. GitHub then cannot evaluate the file's `on:` triggers, so no job runs and the failure surfaces only as a 0-second run with no jobs and no logs. Three changes: - Remove the duplicate key, restoring test.yml to a parseable state. - Add .github/workflows/workflow-lint.yml, running actionlint over .github/workflows. It is a separate file on purpose: a workflow cannot validate itself, so the check that would have caught this is precisely the one that could not run. An independent file still parses and reports a real file:line annotation. Verified against the broken commit: `test.yml:27:9: key "with" is duplicated in element of "steps" section`. - Give every step in test.yml a distinct `name:`. The root cause was four byte-identical `- uses: actions/checkout@v5` anchors: a correct, correctly anchored review suggestion for the new production-browser job was also applied to react-lint, which already had the setting. Unique names remove the ambiguity rather than just detecting its consequences. Also switches `for i in {1..30}` to `for _` to clear the one pre-existing shellcheck finding, so the new gate starts green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`pnpm test:fixtures:generate:0.7.2 -- --output-dir X` forwards the `--` separator through to the Python script verbatim, and argparse treats every argument after a bare `--` as positional. The parser has no positionals, so it exits 2: generate_fixtures.py: error: unrecognized arguments: -- --output-dir X This was latent in #92 rather than new. The job's first CI run passed only because it shared a fixture cache key with the `test` job and got a cache hit, so the generation step was skipped entirely. Giving it a distinct key (the correct fix for that collision) forced a cache miss and ran the step for the first time, exposing the bad invocation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Problem
mainis currently red: run 30333189501 failed in 0s with no jobs and no logs.That is not a test failure. #92 left a duplicate
with:key on thereact-lintcheckout step, sotest.ymlno longer parses:Root cause
CodeRabbit's review was correct and correctly anchored. At the reviewed commit (
2d677c9), line 168 was the newflat-polygon-browserjob's bare checkout, which genuinely neededpersist-credentials: false;react-lintat line 24 already had it.The suggestion got applied to both. Its committable block leads with the context line
- uses: actions/checkout@v5, and this file has four byte-identical checkout steps — so a text-matching edit has no way to know which one was meant.Nothing downstream caught it. Once the file stopped parsing, GitHub could no longer evaluate its
on:triggers, so nopull_requestcheck re-ran on the force-push; the PR kept showing the green run from before the review.Changes
Remove the duplicate key — restores
test.ymlto a parseable state.Add
.github/workflows/workflow-lint.ymlrunning actionlint over.github/workflows.Separate file on purpose, and it should stay that way: a workflow cannot validate itself, so the check that would have caught this is exactly the one that could not run. An independent file still parses when its neighbours are broken, converting a silent 0s run into a real annotation.
Give every step in
test.ymla distinctname:— removes the ambiguity that caused the misapplied edit, rather than only detecting its consequences. Also makes job diffs readable and the Actions UI legible.Fix fixture generation in
production-browser(found by this PR's own CI, see below).Plus
for i in {1..30}→for _, clearing the one pre-existing shellcheck finding so the new gate starts green.The second bug, and why it stayed hidden
With
test.ymlparseable again,production-browserran for real for the first time — and failed:pnpm run <script> -- <args>forwards the--separator to the script verbatim, and argparse treats everything after a bare--as positional. Verified both halves locally:This was latent in #92, not new here. The job's only previously-green run shows
Generate test fixtures -> skipped: it shared a fixture cache key with thetestjob, hit that cache, and never ran the generation step. Giving it a distinct key — the correct fix for the collision CodeRabbit flagged — forced a cache miss and finally executed the bad invocation.Same shape as the first bug, one layer down: a correct review comment, a fix that was right on its own terms, and no gate positioned to see what it uncovered.
Verification
actionlint 1.7.12 run locally against the broken merged commit:
…and against this branch's tree: clean, exit 0. On the first CI run here,
actionlint,react-lint,biome-lintandtestall passed.Follow-up (not in this PR)
mainhas no branch protection and no rulesets — I checked. The 05:55 failure was visible two minutes before the merge and nothing gated it. Requiring these checks would be worth doing separately; it needs a repo-settings change rather than a code change.🤖 Generated with Claude Code